Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feat/fido connect extension #13

Merged
merged 82 commits into from
Apr 30, 2024
Merged

Feat/fido connect extension #13

merged 82 commits into from
Apr 30, 2024

Conversation

PhearZero
Copy link
Member

@PhearZero PhearZero commented Apr 24, 2024 •
edited
Loading

ℹ Overview

This PR includes all of the testing updates and the FIDO Connect Extension

  • Replaces Connect Controller with Signal Controller
  • Moves Connect Attestation to FIDO2 Attestation with Liquid Extension

Android Client: https://github.com/awesome-algorand/android-authentication-client/releases/tag/v0.2.0

📝 Related Issues

Related PRs

✅ Acceptance:

  • Conventional Commits
  • npm run test:cov passes

kylebeee and others added 30 commits April 8, 2024 14:55
@kylebeee
chore/unit-tests-for-assertion-module
3bf36ad 
- added unit tests for assertion module controller
- removed express response object where possible to avoid mocking
@kylebeee
chore/unit-tests-for-attestation-module
ead0b44 
- added unit tests for attestation module controller
- removed express response object where possible to avoid mocking
@kylebeee
chore/unit-tests-for-auth-module
02d273a 
- added unit tests for auth module controller
- removed express response object where possible to avoid mocking
@kylebeee
fix for auth service from removal of base64url dependency
a73975c
@kylebeee
- removed unnecessary await (my mistake, got confused when i mocked a
5124ba2 
  resolved value for it)
- updated endpoints to not JSON.stringify responses ( handled by nest )
- removed unnecessary mocks
@kylebeee
- removed extra dummyOptions declaration & removed assertionService mock
42d9934 
  overrides
@kylebeee
- removed unnecessary mocks
cfee1ba 
- removed JSON.stringify from api responses
@kylebeee
- removed unnecessary JSON.stringify from api endpoint handlers
28534b3 
- removed unnecessary mocks
@kylebeee
added auth service mock
b868528
@kylebeee
fixed a typo in imports and another typo in provider list
04c128e
@kylebeee
removed JSON.stringify from expected unit test results
e88acd1
@kylebeee
removed JSON.stringify from expect function results for attestation
914e9a7 
controller
@kylebeee
removed JSON.stringify from expect assertion controller results
43401df
@kylebeee
fixed a typo in the auth controller spec
63f5596
@kylebeee
final adjustments
de18cc9
@kylebeee
final adjustments, removed await & spread mocks into useValue
460a180
@kylebeee
final adjustments, spread mocks for useValue
b490dc0
@kylebeee
Revert "final adjustments"
b7d83c7 
This reverts commit de18cc9.
@kylebeee
final adjustment, added spread for mocks with useValue
e012ec8
@kylebeee
change toThrowError to ToThrow(HttpException)
dd4acce
@kylebeee
switched toThrowError to toThrow(HttpException) & renamed dummyACJSON…
cfc54d0 
… to dummyAttestationCredentialJSON
@kylebeee
- changed mock args & responses to more verbose naming
3bed7e6 
- changed mock challenges to be more realistic
@kylebeee
changed toThrowError to toThrow(HttpException)
2b12bfa
@kylebeee
broke out mock overrides to hooks instead of inside tests
27a7140
@PhearZero
chore: remove custom tweetnacl and resolve tests
cd8c436
@kylebeee
modified organization of unit tests to be clearer about mocks
f0c2e73
@kylebeee
moved mock overrides into their own beforeEach hook
760e1f2
@PhearZero
wip: liquid fido extension
a64f4f1
@kylebeee
changed hardcoded buffer to randomBytes
d3d4606
@kylebeee
checking on whether we get proper http exceptions back or one large i…
d77ed4e 
…nternal server error
@PhearZero PhearZero mentioned this pull request Apr 24, 2024
Closed
1 task
@PhearZero PhearZero marked this pull request as draft April 24, 2024 15:20
@PhearZero
refactor: remove connect module
b2e10be 
- fixes up tests
- removes lint errors
@PhearZero PhearZero marked this pull request as ready for review April 24, 2024 15:50
HashMapsData2Value
HashMapsData2Value reviewed Apr 25, 2024
View reviewed changes
client-gen.sh Outdated
sed -i "s/\.\/api/\.\/api.js/g" clients/liquid-auth-client-js/src/client/api_test.spec.ts
rm clients/liquid-auth-client-js/src/client/git_push.sh

#java -jar swagger-codegen-cli.jar generate -i http://localhost:3000/docs-json -l kotlin-client -o clients/liquid-auth-client-kotlin
Copy link
Contributor

@HashMapsData2Value HashMapsData2Value Apr 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this comment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping it for if/when we generate the REST clients, the docs/client gen effort is on hold for now

@HashMapsData2Value
Copy link
Contributor

HashMapsData2Value commented Apr 25, 2024 •
edited
Loading

I noticed when running npm run test:cov locally - and after double-checking in the CI jobs - that 6 tests are skipped and that there's an error with Nest

[Nest] 2003 - 04/25/2024, 1:32:43 AM ERROR [AttestationController] Not Found Exception

Is there a reason for this?

@PhearZero
Copy link
Member Author

I noticed when running npm run test:cov locally - and after double-checking in the CI jobs - that 6 tests are skipped and that there's an error with Nest

[Nest] 2003 - 04/25/2024, 1:32:43 AM ERROR [AttestationController] Not Found Exception

Is there a reason for this?

@HashMapsData2Value A few tests are skipped, still working on getting coverage up. We are testing error conditions for better branch coverage and this is the source of the Nest errors

@PhearZero PhearZero force-pushed the feat/fido-connect-extension branch from edd8ba4 to 2791d87 Compare April 29, 2024 13:33
ori-shem-tov
ori-shem-tov approved these changes Apr 30, 2024
View reviewed changes
Copy link
Contributor

@ori-shem-tov ori-shem-tov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after our meeting

@PhearZero PhearZero merged commit 8a89838 into develop Apr 30, 2024
2 checks passed
@PhearZero PhearZero deleted the feat/fido-connect-extension branch May 4, 2024 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HashMapsData2Value HashMapsData2Value HashMapsData2Value left review comments

@ori-shem-tov ori-shem-tov ori-shem-tov approved these changes

@kylebeee kylebeee Awaiting requested review from kylebeee

Assignees

@PhearZero PhearZero

Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@PhearZero @HashMapsData2Value @ori-shem-tov @kylebeee